Skip to content

Fix lutron caseta occupancy sensors#71309

Merged
balloob merged 6 commits intohome-assistant:devfrom
bdraco:lutron_caseta_occ
May 5, 2022
Merged

Fix lutron caseta occupancy sensors#71309
balloob merged 6 commits intohome-assistant:devfrom
bdraco:lutron_caseta_occ

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented May 4, 2022

Proposed change

Fix lutron caseta occupancy sensors

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link
Copy Markdown

Hey there @swails, mind taking a look at this pull request as it has been labeled with an integration (lutron_caseta) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@bdraco bdraco mentioned this pull request May 4, 2022
22 tasks
Copy link
Copy Markdown
Contributor

@swails swails left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I had to cancel my original review as you already implemented the changes I was going to suggest 😆

@chrisaljoudi
Copy link
Copy Markdown
Contributor

They don't have a 'model' either, which is read unconditionally in LutronCasetaDevice init:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 249, in _async_setup_platform
    await asyncio.shield(task)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/binary_sensor.py", line 34, in async_setup_entry
    entity = LutronOccupancySensor(occupancy_group, bridge, bridge_device)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/binary_sensor.py", line 45, in __init__
    super().__init__(device, bridge, bridge_device)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/__init__.py", line 313, in __init__
    model=f"{device['model']} ({device['type']})",
KeyError: 'model'

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

They don't have a 'model' either, which is read unconditionally in LutronCasetaDevice init:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 249, in _async_setup_platform
    await asyncio.shield(task)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/binary_sensor.py", line 34, in async_setup_entry
    entity = LutronOccupancySensor(occupancy_group, bridge, bridge_device)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/binary_sensor.py", line 45, in __init__
    super().__init__(device, bridge, bridge_device)
  File "/usr/src/homeassistant/homeassistant/components/lutron_caseta/__init__.py", line 313, in __init__
    model=f"{device['model']} ({device['type']})",
KeyError: 'model'

I was just about to post the same thing after reading though the diagnostics.

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

So the unique ids here aren't actually unique if you have multiple bridges, but thats another problem

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

I'm going to fix this for now and migrate the unique ids in 2022.6

@@ -65,16 +85,6 @@ def unique_id(self):
"""Return a unique identifier."""
return f"occupancygroup_{self.device_id}"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not actually unique if you have multiple bridges. We need to prefix it with the bridge id as well since it is unique to the bridge.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can migrate them in 2022.6 though as its better to fix this now instead of adding risk

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

@chrisaljoudi Can you test with the latest push?

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

I also uploaded it as a custom_component for easy testing
#71308 (comment)

@chrisaljoudi
Copy link
Copy Markdown
Contributor

chrisaljoudi commented May 4, 2022

@bdraco the custom component is missing the added check in LutronCasetaDevice init (present in the PR here), but I added that and it works.

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

@bdraco the custom component is missing the the added check in LutronCasetaDevice init (present in the PR here), but I added that and it works.

Thanks. I forgot to push the __init__.py to the custom component. Now done in ca6b28113a924fc2ccd6afcd29f18fb0c3fbafc2

@bdraco bdraco marked this pull request as ready for review May 4, 2022 20:57
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 4, 2022

@chrisaljoudi Thanks for testing.

I'm going to need to migrate the unique ids so they are actually unique in 2022.6. Do you mind if I ping you to test that PR in a few days?

Since I don't actually have any of these I'm going to order up a LRF2-OWLB-P-WH so I can test the migration code but it would be better to get a test from someone who uses them in production.

@chrisaljoudi
Copy link
Copy Markdown
Contributor

chrisaljoudi commented May 4, 2022

@bdraco absolutely happy to! Have 3 bridges in use.

@balloob balloob merged commit d67f19f into home-assistant:dev May 5, 2022
@frenck
Copy link
Copy Markdown
Member

frenck commented May 5, 2022

This one is for the next patch release, right? (Asking as it's not tagged for the milestone...)

@bdraco bdraco added this to the 2022.5.1 milestone May 5, 2022
balloob pushed a commit that referenced this pull request May 5, 2022
* Fix lutron_caseta occupancy sensors

* Fix lutron_caseta occupancy sensors

* Make as service since its a group

* merge

* Revert "merge"

This reverts commit 69d19dc.

* model and type not present
@balloob balloob mentioned this pull request May 5, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Caseta Occupancy Sensors Fail

7 participants